Skip to content

Feature | Write TimeOnly instances as time values - #4604

Merged
cheenamalhotra merged 2 commits into
dotnet:mainfrom
edwardneal:feat/timeonly-variant
Oct 8, 2026
Merged

cheenamalhotra merged 2 commits into
dotnet:mainfrom
edwardneal:feat/timeonly-variant

Conversation

@edwardneal

Copy link
Copy Markdown
Contributor

Description

This PR follows up the earlier #4439. At the conclusion of that PR, DateOnly instances were always written as a date type. This one now raises TimeOnly to parity with DateOnly in three cases:

  1. SqlBulkCopy, based upon a TimeOnly instance within a DataRow or a DataTable.
  2. A simple SqlParameter of SqlDbType Variant containing a TimeOnly instance.
  3. When run in a Debug configuration, a user-defined table type parameter, specified via a SqlParameter with its value being an array of SqlDataRecord objects, all with a column of SqlDbType Variant and a row containing a TimeOnly instance.

I also noticed while in the area that although DateTimeVariantTests was performing a number of validity checks against invalid time values, checks against a valid time value were missing. I've added these.

Notes

  • This only opens the DataRow and DataTable paths in SqlBulkCopy; the SqlDataReader path was already open. I opened this path to the other two situations by making an explicit type check in TdsParser.WriteSqlVariantDataRowValue.
  • The simpler case (a Variant-typed SqlParameter) is opened by the change to TdsParser.WriteSqlVariantValue.
  • When run in Debug mode, an assertion was tripping based upon the contents of s_canAccessSetterDirectly. I added a mapping confirming that a variant type can contain a TimeOnly instance.

Issues

None. Repeats the style of fix in #4439 for TimeOnly instances.

Testing

New test cases have been added which verify functionality, existing tests continue to pass.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@priyankatiwari08 priyankatiwari08 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to approve. The change maps TimeOnly through the existing TimeSpan/time serialization paths for both normal and sql_variant writes, keeps the support #if NET-scoped, and adds manual coverage for min/max TimeOnly plus a valid TimeSpan baseline across the affected variants.

I don't see a blocking issue. No public API or ref/ update is needed.

@cheenamalhotra cheenamalhotra moved this from To triage to In review in SqlClient Board Sep 23, 2026
@cheenamalhotra

Copy link
Copy Markdown
Member

/azp run

@cheenamalhotra
cheenamalhotra enabled auto-merge (squash) October 1, 2026 07:02
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.

@cheenamalhotra
cheenamalhotra merged commit 5949e4b into dotnet:main Oct 8, 2026
208 of 213 checks passed
@edwardneal
edwardneal deleted the feat/timeonly-variant branch October 9, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants